Integrate AVPR API int arc-validate #92
Conversation
There was a problem hiding this comment.
Nice progress, but i think we need more separation of the different API layers (see my individual comments).
Additionally, the cli tool usage now gets a bit confusing.
- There must be a flag for testing against an installed preview package (e.g.
arc-validate validate -p test --previewto test agtainst latest installed preview version of packagetest) - Please make
-prexplicit as--previewonly.-pris very similar to-pv - While you are at it, we should make
-v--verboseonly so we can use-vinstead of-pvfor package version
| packageName = packageName | ||
| ) | ||
|
|
||
| static member UninstallPackage( |
There was a problem hiding this comment.
There are now 2 versions of uninstall and list. Are both used in the CLI tool? I see multiple problems with this:
- Uninstall prints "uninstalling all package versions of {packageName}...", but that is not true since the methods uninstall preview/prod packages respectively.
Listshould just list everything (prod/preview side-by-side)
| @@ -256,3 +268,142 @@ type API = | |||
| with e -> | |||
| Error e.Message | |||
|
|
|||
| type AVPR = | |||
There was a problem hiding this comment.
I think here, there should be 3 levels instead of 2 (API/AVPR) in a new ARCValidationPackages.API namespace:
Common: top level functions that are independent of preview/prod (e.g.List)Preview,AVPR: with functions that are named the same but differ internally (e.g.Install).
This can then also be mirrored in ARCValidate.API.PackageAPI for cleaner separation
|
|
||
| type GeneralError(msg : string) = | ||
| inherit Exception(msg) | ||
|
|
||
| type AVPRAPI () = | ||
| member private this.BaseUri = Uri("https://avpr.nfdi4plants.org") |
There was a problem hiding this comment.
is it necessary to use instance methods instead of static methods here?
|
|
||
| static member List( | ||
| args: ParseResults<PackageListArgs>, | ||
| ?Verbose: bool, | ||
| ?Token: string | ||
| ) = | ||
|
|
||
| match ARCValidationPackages.API.GetSyncedConfigAndCache(?Token = Token) with | ||
| match ARCValidationPackages.API.GetSyncedConfigAndCache(false, ?Token = Token) with |
There was a problem hiding this comment.
This means we can only list prod packages, right?
| @@ -63,7 +63,7 @@ module ValidateAPI = | |||
| AnsiConsole.Write(TextPath(Path.GetFullPath(root))) | |||
| AnsiConsole.MarkupLine("") | |||
|
|
|||
| match ARCValidationPackages.API.GetSyncedConfigAndCache(?Token = token) with | |||
| match ARCValidationPackages.API.GetSyncedConfigAndCache(false, ?Token = token) with | |||
There was a problem hiding this comment.
So if i understand this correctly, we can install preview/prod packages, but can only perform validation against prod?
| @@ -22,7 +22,7 @@ let ``ValidateCommand CLI Tests`` = | |||
| testFixture (Fixtures.withToolExecution | |||
| true | |||
| "../../../../../publish/arc-validate" | |||
| [|"-v"; "package"; "install"; "test"; "-pv"; "2.0.0"|] | |||
| [|"-v"; "package"; "install"; "test"; "-pv"; "2.0.0"; "-pr"|] | |||
There was a problem hiding this comment.
So here we install preview version 2.0.0, but below run validate -p test -pv 2.0.0. What if there is prod/preview 2.0.0 installed side-by side? we should only use preview packages here if there is a flag.
I know that 2.0.0 should be equal on preview/prod but it is confusing nonetheless.
|
Closed in favor of resuming from https://github.com/nfdi4plants/arc-validate/tree/avprCli |
No description provided.